Deprecate RedirectsPanel in favor of the HistoryPanel #2223
Conversation
|
Originally I was thinking it would be a message on the template that gets rendered for the |
|
Should I include it in the template too? |
|
@JohananOppongAmoateng I'm good with this as is. I wanted to raise it to see what you thought. |
tim-schilling
left a comment
tim-schilling
left a comment
There was a problem hiding this comment.
This looks good to me. I'd like another @django-commons/django-debug-toolbar-committers to approve of the idea (not necessarily the code) before I merge this in.
|
Hello, I think a message in the template it's not a lot of work and another warning for people who don't read the warning in console. |
|
Mm that's true I will add a warning in the template. Should it be something like a toast? |
Yep only if the user is on the template ofc. |
tim-schilling
left a comment
tim-schilling
left a comment
There was a problem hiding this comment.
The new styles look pretty fixed with pixel units and static colors. This needs to be more flexible for both themes and people who adjust the zoom on the page. The classes also need to be prefixed with the toolbar id (see other styles). This prevents our styles from impacting application styles.
tim-schilling
left a comment
tim-schilling
left a comment
There was a problem hiding this comment.
The latest changes have caused the toolbar's side panel to disappear. I suspect it's due to trying to use the id="djDebug" attribute. My suggestion of using that as a prefix was wrong. This redirect HTML isn't actually within the toolbar's container, though we don't want custom styles floating around. We would need to identify a new container, like djdt-redirect. Though that said, I'm guessing that path is going to be more effort than we want to spend since this is deprecated. I think putting the styles in the <head> may be best. Though I wonder how that's going to go over with a CSP 😬
Log a deprecation warning and include warning on the panel itself. It asks users to comment on the GitHub issue if the redirects panel is still a requirement for them.
tim-schilling
force-pushed
the
deprecate-redirects-panel
branch
from
edf9a91 to
3837b0c
Compare
tim-schilling
changed the title
|
Thank you for making those changes @JohananOppongAmoateng! I removed the toolbar.css from the redirects template, cleaned up the indentation on the styles, squashed the commits and adjusted the commit message. |
3 participants
Description
Please include a summary of the change and which issue is fixed. Please also
include relevant motivation and context. Your commit message should include
this information as well.
Fixes #2216
Checklist:
docs/changes.rst.